Keep a by-ref writeback's native type only when the written-back type fits it by SanderMuller · Pull Request #6564 · phpstan/phpstan-src

SanderMuller · 2026-09-23T14:40:58Z

A regression in 2.2.15 from #6463. With treatPhpDocTypesAsCertain: false, this reports an error that 2.2.14 did not:

function offsetCapture(string $s): ?string
{
	if (! preg_match('/(a)(b)?/', $s, $m, PREG_OFFSET_CAPTURE)) {
		return null;
	}

	return isset($m[2]) ? $m[2][0] : null;
}

Offset 2 on *NEVER* in isset() always exists and is not nullable.

array_key_exists(2, $m) and $m[2] ?? null report the same way. I found no issue for it.

Cause. Since #6463, the native type after a by-ref argument is the parameter's own type. For a builtin, that is its signature map entry, and preg_match's &$matches is string[]. With PREG_OFFSET_CAPTURE each element is an array{string, int}, so the guard intersects the offset-capture shape with array<string>, and the native type becomes *NEVER*. Without the flag the elements are strings, which is why only this case breaks. Bisected: 3f97b5a57 is clean and 091f2cfc4 is the first bad commit.

The same happens to a userland @param-out that contradicts its declaration. PHP checks a by-ref parameter's type only on the way in. So int &$v with @param-out string really does hold a string afterwards, but the native type said int.

Fix. Keep the parameter's own type as the native type only when the written-back type is a subtype of it. Otherwise fall back to mixed. Where the two agree, as in #6463's bug-15250.php cases, nothing changes.

Trade-off. The fallback is mixed, not the nearest type that fits. After a PREG_OFFSET_CAPTURE match, a later is_array($m) is therefore no longer reported as always true with treatPhpDocTypesAsCertain: false. 2.2.14 did report it. A union with the written-back type would keep that. But it would put the PHPDoc shape back into the native type, which is what #6463 set out to stop.

Tests.

nsrt/by-ref-writeback-native-type.php: offsetCapture() and userlandParamOut() fail without the change, with *NEVER* and int. noFlags() passes either way and guards the case that must not change.
IssetRuleTest::testPregMatchOffsetCaptureWithoutTreatPhpDocTypesAsCertain() fails without the change with the reported error.

What else I checked.

The native type after sort, preg_match without flags, preg_match_all, str_replace, exec, parse_str, similar_text, array_push and settype is the same before and after.
A 4.5k-file project analysed with treatPhpDocTypesAsCertain: false gives the same 3106 errors before and after.
make phpstan is clean. In make tests, the only failure is MissingCheckedExceptionInMethodThrowsRulePhp74Test::testInternalErrors, which fails the same way on 2.2.x without this change on my machine.
On 2.3.x the same code sits in ArgumentsHandler.php, so the merge-up needs the same change there.

🤖 Generated with Claude Code

ondrejmirtes · 2026-09-23T19:18:41Z

 						$byRefNativeType = $currentParameter instanceof ExtendedParameterReflection
 							? $currentParameter->getNativeType()
 							: $byRefType;
+						if (!$byRefNativeType->isSuperTypeOf($byRefType)->yes()) {


We don't need to evaluate this for the ternary else (when $byRefType is assigned to $byRefNativeType). Feel free to write an ordinary if instead of the ternary above.

Done in 3ee2eae.

… fits it After a by-reference argument, the native type of the variable became the parameter's own type: its declaration, or for a builtin its signature map entry. PHP checks a declaration only on the way in and a signature map entry not at all, so neither describes what the call writes back when the two disagree. preg_match() with PREG_OFFSET_CAPTURE writes arrays into a slot the signature map declares as string[]. The guard `if (! preg_match(...))` then intersected the offset-capture shape with array<string>, the native type became never, and with treatPhpDocTypesAsCertain: false isset($m[2]) reported "Offset 2 on *NEVER* in isset() always exists and is not nullable". The same happens to a userland @param-out that contradicts its declaration. The native type now falls back to mixed when the written-back type is not a subtype of the declaration. Where they agree, as in phpstan#6463's own cases, nothing changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

When the parameter is not an ExtendedParameterReflection, the native type is the written-back type itself, and the check has nothing to decide. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ondrejmirtes · 2026-09-24T08:12:21Z

Thank you.

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96034 Tested-by: core-ci <typo3@b13.com>

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96033 Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de> Tested-by: Oliver Klee <typo3-coding@oliverklee.de> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Benni Mack <benni@typo3.org> Reviewed-by: Sascha Nowak <typo3@saschanowak.me> Tested-by: core-ci <typo3@b13.com>

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96035 Tested-by: core-ci <typo3@b13.com>

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96033 Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de> Tested-by: Oliver Klee <typo3-coding@oliverklee.de> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Benni Mack <benni@typo3.org> Reviewed-by: Sascha Nowak <typo3@saschanowak.me> Tested-by: core-ci <typo3@b13.com>

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96034 Tested-by: core-ci <typo3@b13.com>

Updates the PHPStan development dependency from `^2.2.14` to `^2.2.15`, keeping all maintained branches on the same version. PHPStan v2.2.15 knows that `vsprintf()` only throws a `ValueError` [1] and reports the `ArgumentCountError` catch in `LogDataTrait` as dead catch on all branches, which is valid and therefore removed. On main and v14.3, which disable `treatPhpDocTypesAsCertain` globally, it further reports two false positives caused by a regression of the native type of by-reference arguments [2][3], which is fixed upstream [4] but not released yet: * The null coalescing on the `scope` match in `CorrelationId::fromString()` is redundant anyway, because `PREG_UNMATCHED_AS_NULL` always provides the offset. It is removed on all branches. * The `is_array()` check after the workspace overlay in `SuggestWizardDefaultReceiver` is required, because `BackendUtility::workspaceOL()` can set the record to false. It is added to the PHPStan baseline of main and v14.3 until the upstream fix is released, which will then report the unmatched baseline entry for removal. [1] phpstan/phpstan-src#6517 [2] phpstan/phpstan-src#6463 [3] phpstan/phpstan#15297 [4] phpstan/phpstan-src#6564 Executed commands: > Build/Scripts/runTests.sh -s composer -- \ require --dev phpstan/phpstan:^2.2.15 > Build/Scripts/runTests.sh -s phpstanGenerateBaseline Resolves: #110772 Releases: main, 14.3, 13.4 Signed-off-by: Stefan Bürk <stefan@buerk.tech> Change-Id: Id0314ca79afa609489d4b2bfae7e64a9d9f7ea4a Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/96035 Tested-by: core-ci <typo3@b13.com>

SanderMuller mentioned this pull request Sep 23, 2026

Ask Composer's autoloaders for the file instead of letting them include it #6431

Open

ondrejmirtes requested changes Sep 23, 2026

View reviewed changes

SanderMuller requested a review from ondrejmirtes September 23, 2026 19:53

SanderMuller and others added 2 commits September 24, 2026 10:11

Check the by-ref native type only when the parameter declares one

23da8ed

When the parameter is not an ExtendedParameterReflection, the native type is the written-back type itself, and the check has nothing to decide. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ondrejmirtes force-pushed the by-ref-native-type-internal branch from 3ee2eae to 23da8ed Compare September 24, 2026 08:11

ondrejmirtes approved these changes Sep 24, 2026

View reviewed changes

ondrejmirtes merged commit d27106a into phpstan:2.2.x Sep 24, 2026
238 of 239 checks passed

Navigation Menu

Keep a by-ref writeback's native type only when the written-back type fits it - #6564

Keep a by-ref writeback's native type only when the written-back type fits it#6564
ondrejmirtes merged 2 commits into
phpstan:2.2.xfrom
SanderMuller:by-ref-native-type-internal

SanderMuller commented Sep 23, 2026

Uh oh!

ondrejmirtes Sep 23, 2026

Uh oh!

SanderMuller Sep 23, 2026

Uh oh!

Uh oh!

ondrejmirtes commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SanderMuller commented Sep 23, 2026

Uh oh!

ondrejmirtes Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

SanderMuller Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ondrejmirtes commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants